fix(codex): handle remote request_user_input approvals#382
fix(codex): handle remote request_user_input approvals#382hqhq1025 merged 1 commit intotiann:mainfrom
Conversation
There was a problem hiding this comment.
Findings
- [Major] Explicit deny for
request_user_inputis downgraded to cancel.handleUserInputRequest()rejects the non-accept path, and this launcher catch always returns{ decision: 'cancel' }, so Telegram or any genericdenyPermission()caller can never propagatedeclineback to Codex. The same path also records the completed request as canceled instead of denied incli/src/codex/utils/permissionHandler.ts. This changes user-visible behavior for deny actions instead of only fixing the hang. Evidence:cli/src/codex/codexRemoteLauncher.ts:519,cli/src/codex/utils/permissionHandler.ts:168,hub/src/telegram/callbacks.ts:90.
Suggested fix:type UserInputApproval = | { decision: 'accept'; answers: RequestUserInputAnswers } | { decision: 'decline' | 'cancel'; reason?: string } const result = await permissionHandler.handleUserInputRequest(id, input) return result.decision === 'accept' ? result : { decision: result.decision }
Summary
- Review mode: initial
- 1 major finding. Main risk is that explicit deny paths for remote
request_user_inputprompts no longer behave like deny; they are collapsed into cancel/abort semantics instead.
Testing
- Not run (automation environment missing
bun;bun x vitest ...andbun run typecheckboth failed with/bin/bash: bun: command not found)
HAPI Bot
| const message = error instanceof Error ? error.message : String(error); | ||
| logger.debug(`[Codex] request_user_input failed: ${message}`); | ||
| return { | ||
| decision: 'cancel' |
There was a problem hiding this comment.
[MAJOR] Explicit deny is being collapsed to cancel here. handleUserInputRequest() rejects every non-accept outcome, and this catch hard-codes { decision: 'cancel' }, so Telegram or any other denyPermission() caller never returns decline to Codex. The same path is then finalized as canceled instead of denied in cli/src/codex/utils/permissionHandler.ts:168, which changes the behavior of real deny actions rather than only fixing the hang.
Suggested fix:
type UserInputApproval =
| { decision: 'accept'; answers: RequestUserInputAnswers }
| { decision: 'decline' | 'cancel'; reason?: string }
const result = await permissionHandler.handleUserInputRequest(id, input)
return result.decision === 'accept'
? result
: { decision: result.decision }
hqhq1025
left a comment
There was a problem hiding this comment.
Looks good — the root cause is real and the fix is correct. I traced the full approve flow end-to-end and it works.
Re: the bot finding about "deny downgraded to cancel" — this is consistent with existing behavior. All tool denials from Telegram (which calls denyPermission without a decision parameter) end up as cancel via mapDecision('abort'). The web UI's RequestUserInputFooter has no deny button, so this path is only reachable from Telegram. Not a regression.
Two minor suggestions (non-blocking):
1. Status label inversion in handlePermissionResponse
// Current (semantically inverted):
status: response.approved ? 'denied' : 'canceled',
// Should be:
status: response.approved ? 'canceled' : 'denied',When approved: true but answers are empty, it's a system-level cancellation (not a user denial). When approved: false, it's a user denial. The current labels are swapped.
2. No onComplete on the deny path
The approve path calls onComplete (which sends tool-call-result to the web UI), but the deny path rejects without calling onComplete. This could leave the web UI tool card visually stuck in "pending" when denied from Telegram. Consider adding an onComplete call in the deny branch to match the regular tool pattern.
Both are minor — happy to see these addressed in a follow-up if you prefer.
Fix remote Codex sessions hanging/canceling on
request_user_inputtool approvals.Root cause:
codexRemoteLauncherregisters app-server permission handlers withoutonUserInputRequestappServerPermissionAdapterfalls back tocancelforitem/tool/requestUserInputThis patch wires
request_user_inputinto the existing Codex remote permission flow and adds focused tests.Validated with:
bun x vitest run src/codex/utils/permissionHandler.test.ts src/codex/utils/appServerPermissionAdapter.test.ts src/codex/codexRemoteLauncher.test.tsbun run typecheck